-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Link to "run the fullscreen rendering steps" in Fullscreen #2763
Conversation
@zcorpan, PTAL? "rendering steps" is a bit of a misnomer as it only fires events, if you have a better name I'll take it. |
c503df8
to
ca65ca4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. No strong opinion about the name of the hook, but agree that "rendering steps" seems a bit weird.
How about just "run the fullscreen steps"? Or "run the fullscreen animation frame steps"? |
run the fullscreen steps SGTM. |
In CSSOM View I say "This section integrates with the event loop defined in HTML. [HTML]" before the relevant algorithm to clarify what it's for. |
Matches whatwg/fullscreen#94. Fixes the fullscreen part of #707. Drive-by: re-wrap lines to 100 columns.
ca65ca4
to
c1b088a
Compare
Renamed in whatwg/fullscreen#94 and updated this to match, LGTY? |
Didn't add "Editorial:" to this one since it actually adds a real code path to HTML instead of calling into the void. |
The fullscreen PR was reviewed and merged, so I'll just go ahead with this, just a trivial rename since review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Introduced in whatwg/fullscreen#92.
Fixes the fullscreen part of #707.
Drive-by: re-wrap lines to 100 columns.